Skip to content
This repository has been archived by the owner on Jun 16, 2021. It is now read-only.

Revert change in a75b53 to make ValidRemotes private #73

Merged
merged 1 commit into from
Oct 23, 2015

Conversation

ibuildthecloud
Copy link
Contributor

This constant was in use by code using libcompose

Signed-off-by: Darren Shepherd darren@rancher.com

This constant was in use by code using libcompose

Signed-off-by: Darren Shepherd <darren@rancher.com>
@dnephin
Copy link
Contributor

dnephin commented Oct 7, 2015

Could we expose a better interface than a list of constants? Why does the code need to know about this?

@ibuildthecloud
Copy link
Contributor Author

@dnephin Hmmm... The reason the code is here is that you can build from any valid Docker remote location, https://docs.docker.com/reference/api/docker_remote_api_v1.20/#build-image-from-a-dockerfile

Looking at the Docker compose docs doesn't seem like this is possible. Is there a specific reason you aren't allowed to do a Docker build from a remote location? By remote I mean the definition of the remote param on POST /build

@dnephin
Copy link
Contributor

dnephin commented Oct 7, 2015

No reason, there's an PR to add that support (that needs more work), but I don't know if anyone is actively working on it.

There's been very little demand for it, so it hasn't been a priority.

@dnephin
Copy link
Contributor

dnephin commented Oct 7, 2015

Sorry, my earlier question was, why does code external to libcompose need this constant

@ibuildthecloud
Copy link
Contributor Author

@dnephin The reason why is that you can supply a custom Builder. So currently we use libcompose in a server side context. In that context we don't want to allow builds that reference the disk. So we only allow remote builds if libcompose is ran in a server side context.

@vdemeester
Copy link
Collaborator

@ibuildthecloud Using it on rancher-compose right ?

I'm wondering something though, should it be there (in libcompose) or in docker/docker ? I would think in docker/docker in a non-internal package (like the api one that should be there at some point or in a pkg — but I'm not that sure about pkg).

@ibuildthecloud
Copy link
Contributor Author

In docker/docker would be preferred and that is where I copied the list to begin with. Let me look from where. I imagine it was private.

@vdemeester
Copy link
Collaborator

@ibuildthecloud @dnephin I think we should fill an issue for make this better and merge this one as it breaks current codes, wdyt ? (the actual list feels weird at it would mean git@github.com:vdemeester/libcompose is a valid git+ssh remote but vincent@myhost:src/docker/libcompose would not 😓 — but that's outside the scope of this PR).

@ibuildthecloud isn't it in pkg/urlutil (https://github.com/docker/docker/blob/master/pkg/urlutil/urlutil.go#L11) ?

@dnephin
Copy link
Contributor

dnephin commented Oct 22, 2015

Ya, I'm ok merging this, it shouldn't hurt anything

@vdemeester
Copy link
Collaborator

Created #95, so LGTM 🐺

vdemeester added a commit that referenced this pull request Oct 23, 2015
Revert change in a75b53 to make ValidRemotes private
@vdemeester vdemeester merged commit f035280 into docker:master Oct 23, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants